Skip to content

Fix type disagreement for required_headers in service code#145

Merged
daneah merged 1 commit intodevfrom
fix/required-headers-typing
Jun 4, 2025
Merged

Fix type disagreement for required_headers in service code#145
daneah merged 1 commit intodevfrom
fix/required-headers-typing

Conversation

@daneah
Copy link
Member

@daneah daneah commented Jun 4, 2025

This change is a: (check at least one)

  • Bugfix
  • Feature addition
  • Code style update
  • Refactor
  • Release activity

Is this a breaking change? (check one)

  • Yes
  • No

Is the:

  • Title of this pull request clear, concise, and indicative of the issue number it addresses, if any?
  • Test suite passing?
  • Code coverage maximal?
  • Changelog up to date?

What does this change address?

Consumers of apiron could run into type errors when overriding required_headers in a service they define.

  • The required_headers attribute in ServiceBase is writeable, but consuming code that tried to override required_headers as a property would make required_headers read-only, which breaks the Liskov substitutability principle.
  • The type of the required_headers attribute in ServiceBase was not as restrictive as the return type of the required_headers property in ServiceMeta, potentially leading to type disagreements in consuming code

How does this change work?

  • Update required_headers in the ServiceBase class to be a property, matching the definition in ServiceMeta and making it overridable as a property in consuming code.
  • Ensure the return type is as restrictive in ServiceBase as in ServiceMeta.

Copy link
Contributor

@NazimHAli NazimHAli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@daneah daneah merged commit 37708b4 into dev Jun 4, 2025
10 checks passed
@daneah daneah deleted the fix/required-headers-typing branch June 4, 2025 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants